fix(render): disable browser scroll anchoring on the table holder - #4935
Draft
lukecotter wants to merge 4 commits into
Draft
fix(render): disable browser scroll anchoring on the table holder#4935lukecotter wants to merge 4 commits into
lukecotter wants to merge 4 commits into
Conversation
Chrome's scroll anchoring adjusts scrollTop when the virtual renderer inserts rows above the viewport, double-compensating against the renderer's own paddingTop/scrollTop management and causing drift on scroll-up. Set overflow-anchor:none on .tabulator-tableholder, the standard approach for JS-managed virtual scrollers.
e2e guard that .tabulator-tableholder computes overflow-anchor:none, so the scroll-anchoring fix can't silently regress out of the built CSS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Chrome's scroll anchoring competes with the virtual renderer's padding writes. At the
bottom of a long list it pins
scrollTopand the two fight in an infiniteoscillation — the renderer adjusts padding, anchoring restores the scroll offset, and
the cycle repeats every frame.
Fix
Set
overflow-anchor: noneon the table holder. The virtual renderer positions contentitself, so browser anchoring has nothing useful to contribute.
Test
test/e2easserts the table holder disables scroll anchoring.This PR also corrects the
scroll-jumpe2e measurement, because with anchoring off theexisting measurement no longer works and, once corrected, it reports a real defect on
current master. Two changes, in the last two commits:
step, so that row must be in the DOM before and after. On master anchoring cancels most
of every step — the test asks for 250px and gets 46, 14, 38, 54 — so the row survives by
accident. With anchoring off the scroll does what it was asked, the whole window turns
over, and no row is present in both states, so nothing is measured. The step is now
80px, which keeps a trackable row.
moved − scrolled, wherescrolledis the holder's ownscrollTopdelta. A virtual renderer revises its totalscrollHeightas it measuresreal row heights, and
scrollTopis read from the top of that changing document, so itshifts by the height correction while nothing on screen moves. It now compares the
row's screen movement against the requested scroll distance, which has no such term.
Both tolerances, 20px and 5px, are unchanged. Verified against three other states: the new
metric still fails at 280px on the code before the #3654 fix, so it keeps that coverage; it
passes on this branch; and it fails at 124px on current master, where anchoring silently
cancels part of each requested scroll. That last figure is independent evidence for this
fix — the defect was previously invisible because the measurement absorbed it.
Performance
Neutral. 500k rows, K=5, medians: